feat: multi-provider pull requests page with in-app reviews - #4849
feat: multi-provider pull requests page with in-app reviews#4849Bil0000 wants to merge 199 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Reviewed the new Effect services (GitHubPullRequestCli, PullRequestService), the new contract errors, and the touched call sites. Module layout, namespace imports, Context.Service + inline interface, make/layer, and dependency acquisition via yield* Foo.Foo all follow the conventions. The findings below are all in error modelling.
Posted via Macroscope — Effect Service Conventions
fc40c4f to
8912648
Compare
8912648 to
4fcb7b9
Compare
4fcb7b9 to
e1fca58
Compare
Models a GitHub pull request for the upcoming pull requests page: list entries, detail with checks/comments/commits, diff, and the merge, ready, draft, close, reopen and comment operations. Two error shapes rather than one: PullRequestUnavailableError covers the states that switch the whole feature off (no gh, logged out, non-GitHub remote) so the UI can explain the fix, while PullRequestOperationError carries per-request failures.
Normalizes the gh payloads into the contract shapes. Enum-ish fields decode as plain strings and are mapped here, so a gh release that adds a check conclusion or review state cannot fail a whole payload, and a malformed row is skipped rather than blanking the batch. Also folds reviews into the comment list (dropping bodyless approvals, which the review decision already reports) and reads unresolved review threads, which gh pr view cannot return.
Wraps the gh invocations the page needs, reusing the existing process wrapper rather than adding a second one. Two of them needed capabilities GitHubCli.execute did not expose: - comment bodies travel over stdin, because argv is visible in process listings and is echoed back inside process-runner failure messages - pr diff raises the output cap to 8 MiB and reports truncation instead of failing on a large pull request Review threads go through gh api graphql, since gh pr view --json has no field for them.
Resolves each project to its GitHub repository through the repository identity already stored on the project, so no extra remote lookup is needed, and lists repositories once even when several worktrees share one. Failure handling is deliberately split: an unreachable repository becomes an entry in the result's errors so healthy repositories still render, while a missing or logged-out gh stops the whole listing, because it is not repository-specific. The repository travels through the client, so it is checked against the project's own remote before reaching gh --repo.
Reads reuse a recent result and refresh explicitly, since every one of them shells out to the GitHub CLI. Mutations run serially per environment: gh actions on the same pull request are order-sensitive and the detail view refetches after each one.
One place resolves how a pull request state reads, so no surface can drift: draft outranks conflicts, because a draft is not heading for a merge yet. Involvement filtering and grouping run over the state's superset returned by the server, so switching between All, Reviewing and Authored never waits on the network. The unavailable state reads the server's message to name the fix — install gh, sign in — rather than reporting a generic failure.
The markdown renderer has no element for a video, so an embed would show up as a bare link. Splits a body into markdown runs and the two video shapes GitHub itself produces: a video or source tag, and a bare link on its own line to a video file or an uploaded attachment. Three cases stay markdown on purpose: anything inside fenced code, an image drop written as an image, and a source that is not http(s).
Summary, Timeline and the gh-backed actions: merge with the repository's allowed methods, ready, draft, close, reopen, comment, copy link, open on GitHub. Merge and close confirm first. Fix findings and Resolve conflicts hand the work to a thread: both check the pull request out into its own worktree, open a thread there, and hand over a task-specific prompt. Everything quoted into those prompts is bounded and marked untrusted, since review bodies and check output are attacker controlled on a public repository. The timeline reports a merge rather than the close GitHub records alongside it, which would otherwise misstate what happened.
Renders the patch through the same viewer as the thread diff panel, and lazily, so its worker pool only ships once the tab is opened. The viewer renders at its natural height and expects its host element to scroll, which is the contract the diff panel already relies on. Deliberately not the annotatable wrapper: that one writes review comments into a thread's composer draft, and this page has no thread. A patch the viewer cannot structure falls back to raw text rather than an empty tab.
Reachable from the sidebar. Filters live in the URL so a view can be shared, while the page size stays local: a shared link should open the first page. Loading further results holds the last page on screen while the larger one arrives, so the list grows underneath instead of falling back to skeletons, and a sentinel starts the next page before it scrolls into view. gh pr list exposes no cursor, so this re-reads a larger page rather than continuing from an offset — cheap at the sizes a pull request list reaches.
A pull request link in a thread, the sidebar or the branch toolbar now lands on the in-app page, which offers the browser as one of its actions. The page resolves the owning project from the repository, so the link only needs the repository and number and no call site has to change. Anything that is not a GitHub pull request URL — a GitLab merge request, an unrelated host — still goes straight out to the system browser.
Locks in the behaviour that is easy to regress and invisible from the UI: non-GitHub projects are skipped, worktrees sharing a repository read it once, one unreachable repository leaves the healthy ones listed while a missing CLI stops the whole listing, entries order by most recent update, a review request counts for the viewer but not on their own pull request, and a repository that does not belong to the project never reaches gh. Also names the conversation page size the truncation flag was comparing against.
A closed pull request was grey here and red in the thread badge, so one pull request read as two different things in two places. The page now uses the same ink for open, closed and merged; draft and conflicts are states the badge never shows. The meta line was reading children with Array.isArray, which is wrong for a single child or a fragment. Children.toArray handles both, drops the nullish segments, and lets a separator borrow the key of the segment it precedes.
The method is a preference for the merge action, not five separate actions, and the project filter in the same feature already expresses that with a radio group. Replaces the hand-written "(selected)" suffix.
Addresses the review of the server error model and gh decoding: - PullRequestUnavailableError derives its message from `reason` instead of copying the CLI's text, and now carries the wrapped failure as `cause`, which the cli-missing and cli-unauthenticated branches were discarding. - An unusable gh response reports the read it came from. The shared decode helper reused one tag whose message was hard-coded to getPullRequest, so every read misreported itself. - An empty viewer login gets its own error rather than a JSON decode tag with a synthetic cause; nothing failed underneath it. - Repository merge settings are required, not optional-defaulting-to-true. They are requested together, so a partial response now fails instead of offering a merge method the repository forbids. - Team review requests no longer enter reviewRequestLogins. The viewer check compares those against a login, so a team slug could read as the viewer. - Truncation is measured on the raw row count. It was read after tolerant decoding, so one malformed row could end pagination early.
The state was substring-matching the error text to recover which of the three reasons it was looking at. The server now derives a stable sentence from the reason itself, so this renders it.
Three body-segmentation bugs from review: - A video tag on a line with text around it replaced the whole line, so the prose disappeared. Only a tag that owns its line is an embed now. - A fence closed on any marker, so a ~~~ line ended a ``` block and exposed its contents to the video rules. The opening marker is tracked instead. - Runs were trimmed on both ends, which drops the four leading spaces that open an indented code block. Only blank lines around a run go now.
The filter only matched whole reviews, so review-thread roots — the inline feedback the detail service labels review-comment — never reached the prompt, which could then report no findings on a pull request full of them. Each finding now names its file, which is the point of an inline comment.
The clipboard write was unawaited for failure, so a denied clipboard threw past the success toast and left the handoff button stuck — with the worktree and the thread already created. The two outcomes are now reported apart.
…t3code into feat/pull-requests-page
maria-rcks
left a comment
There was a problem hiding this comment.
Three user-visible state and pagination findings from reviewing the new pull-requests page. Each inline comment includes the failure mode and a focused test/fix direction.
…reloads Address Maria's review on the pull requests page: - Merge diff-stat batches by row identity so counts on screen survive the keyed stats query going pending when rows are added or removed. - Read the Authored and Review requested partitions from their own server-filtered queries so the priority view is complete up front and a continuation only appends below the reader instead of regrouping loaded pages above them. - Persist a bounded per-environment snapshot of the last answered list and hydrate it after a reload, so a revisit renders retained rows and reconciles them in place instead of cold-starting into skeletons.
…drating A cast trusted whatever localStorage held, so one malformed row — or a snapshot from an older schema — would crash the list on every reload. The snapshot is now decoded with the contract's own schema and rejected whole when it does not fit, falling back to the cold start it would have broken.
…view-comments # Conflicts: # apps/web/src/components/sidebar/SidebarChrome.tsx # apps/web/src/routeTree.gen.ts
…destroy edits The cleanliness check before moving a reused worktree onto a rewritten pull request head is a snapshot: another thread can edit a tracked file between it and the reset, and --hard would silently discard that edit. --merge behaves identically on a clean tree but git itself refuses the move when it would overwrite an unstaged change, so the race loses nothing — the refresh fails, is logged, and the worktree is handed back stale.
The list borrowed the chat/settings 2.5rem fade band and matched it with pt-10, which read as a large empty strip under the header. The page gets its own 1.5rem band with matching padding.
Opening a PR squashed the list: the panel shared the chat preview's storage key and 540px default. The PR page now persists its own width and starts at 50% of the window.
The title/branch block always sat above the tab content and starved the diff of vertical space. On the Code tab the title folds into the compact top row and the files count + diffstat move into the tabs row.
The review footer permanently reserved a strip below the diff. It is now a glass pill trigger (with a pending-comment count) that opens the form as a glass overlay above the code, closing on submit.
Every read leaves the process for a host CLI that can take seconds to tens of seconds, and the 15-60s cache windows meant nearly every page visit paid that clock again — slow enough that the client's own timeout interrupted the authored/reviewing partition reads, which is how authored pull requests went missing from the list. The last success per key is now served instantly inside a stale window while the cache refreshes in the background, and a host's signed-in login is remembered instead of asked per read. Explicit refresh and mutations still punch through via the existing epochs.
The viewer memoizes each visible file's header/annotation portal on the render props and options; inline literals gave them a fresh identity every render, so any state change — a drag-selection, a draft keystroke — rebuilt every portal on screen. They are now stable via useCallback/useMemo.
Opening the Code tab serialized detail fetch, lazy chunk download, and diff fetch. The panel now preloads the chunk and starts the default diff read alongside the detail query, so the tab mounts into a warm cache.
…eat/pull-requests-page
The Comments section read oldest-first with the latest buried under a 'show earlier' fold. It now defaults to newest first with the same order toggle the timeline wears.
Three list fixes: - A refreshed page inherited the previous row order and filed newly opened pull requests at the bottom; a whole-page answer now stands in the host's own order, so the latest lands on top. - The Authored/Reviewing groups grouped only the feed's first page until their own server reads answered, which on a slow host meant authored rows trickling in via infinite scroll. The groups' last answers now persist in the list snapshot and stand in from the first paint. - A deep link carrying an explicit projectId no longer fails to open when the repository identity cannot be inferred.
A local formatter run under the wrong Node build rewrapped this unrelated selector; CI's formatter wants it as it was.
The toggle sat on its own line under 'Comments'; it now rides the heading row itself, as a sibling of the collapse trigger since a button cannot nest a button.
…menu gh pr view --json commits pages from the start, so a pull request with more than a hundred commits never showed its newest ones — the scope dropdown topped out days behind the branch. The review-threads GraphQL read now asks for commits with 'last' and carries the full commit shape, and its list stands in for the gh one wherever it came back non-empty. The dropdown already orders newest first.
Adds a dedicated Pull Requests page: browse, read, review, and act on change requests from every host a workspace uses, without leaving T3 Code.
Summary
A sidebar entry opens a full page listing pull requests across all projects in the environment, with a detail panel beside it for reading, reviewing and acting on one. Four hosts are supported — GitHub, GitLab, Bitbucket and Azure DevOps — behind a single provider interface.
Each host runs through a wrapper that already existed in this repo (
gh,glab, the Bitbucket HTTP client,az), so there is no new authentication story and nothing new to configure.Design
One interface per host.
PullRequestProviderApidefines the operations; the service knows only the registry, so no adapter knows another exists. Adding a host is one file.Capabilities are declared, not assumed. Each provider states what it supports — patch, comment, actions, merge strategies, review verdicts — and the surface follows it. Nothing renders a control that would fail: Azure DevOps has no Code tab because
azexposes no patch, GitLab offers no Request changes because GitLab has no such verdict, and Bitbucket offers no Reopen because Bitbucket has no such endpoint. The server refuses an undeclared operation independently of the UI.A review is one request. The summary, every line comment and the verdict travel together, so a half-written review is invisible to everyone else. GitHub takes that natively; GitLab and Bitbucket have no pending review, so the provider replays it as the requests it is made of, with the verdict last — a review that fails part-way is never an approval.
Hosts, not provider kinds. github.com and a GitHub Enterprise install are the same kind and different accounts, so the viewer, de-duplication and every row key are scoped by host.
Degrade, never blank. One unreachable repository becomes an entry in
errorswhile the rest render. A malformed row is skipped rather than failing its batch. Enum-like fields decode as strings and are normalised, so a host adding a new status cannot break a payload.Scope
What each host allows
Azure DevOps exposes no patch through
az, so it has no lines to write against.Testing
Repo-wide typecheck and lint clean. ~450 tests cover the service, the four decoders, and the invocations each wrapper makes — including the exact request body a review submission produces on each host.
Response shapes for GitHub, GitLab and Bitbucket were verified against the live APIs rather than documentation, which is what caught Bitbucket's page ceiling of 50 (above it the API returns an empty page and no error), its redirect-served diff endpoints, and the absence of a reopen endpoint in its published OpenAPI spec. GitHub's review-submission payload was verified against the live endpoint using a pending review, which was then deleted. Azure DevOps refuses anonymous REST, so its decoding follows the schema already in this repo and stays tolerant of missing fields.
Not exercised end to end: Bitbucket and Azure DevOps write operations, and GitLab's discussion endpoints. No credentials for any of them were available while building; reads are shape-verified where the API allows it and writes are unit-tested at the invocation level only.
Notes for reviewers
T3CODE_BITBUCKET_EMAIL+T3CODE_BITBUCKET_API_TOKEN), which are pre-existing and server-wide rather than per user.Note
Add multi-provider pull requests page with in-app review panel
/pull-requestsroute with filtering (state, involvement, host, project), search, grouping, and a right-panel detail view supporting tabs for summary, timeline, and diff.PullRequestService.GitManager.preparePullRequestThreadnow refreshes an existing PR worktree to the current head commit (fast-forward or reset when safe) and conditionally re-runs the setup script.[!NOTE],[!WARNING], etc.) render as styled callouts in markdown.RightPanelStorestorage version bumps to 10; existing persistedpull-requestsingleton tabs are migrated to per-PR keyed tabs on first load.Macroscope summarized bf82669.